module: refactor and clarify async loader hook customizations #60278

joyeecheung · 2025-10-16T12:40:00Z

Paving the way for #59666, this patch does not introduce observable changes, mostly clarifying which paths are sync/async, or run on the async loader hook thread/on the non-async-loader-hook thread, and updating the comments so that it's easier to sync-ify the paths later.

This updates the comments that assume loader hooks must be async
Differentiate the sync/async loader hook paths in naming #customizations is now #asyncLoaderHooks to make it clear it's from the async APIs.
Differentiate the paths running on the loader hook thread (affects the loading of async other loader hooks and are async) v.s. paths on the main thread calling out to code on the loader hook thread (do not handle loading of other async loader hooks, and can be sync by blocking).
- Hooks is now AsyncLoaderHooksOnLoaderHookWorker
- CustomizedModuleLoader is now AsyncLoaderHooksProxiedToLoaderHookWorker and moved into lib/internal/modules/esm/hooks.js as it implements the same interface as AsyncLoaderHooksOnLoaderHookWorker
- HooksProxy is now AsyncLoaderHookWorker
- Adjust the JSDoc accordingly
Clarify the "loader worker" as the "async loader hook worker" i.e. when there's no async loader hook registered, there won't be this worker, to avoid the misconception that this worker is spawned unconditionally.
The code run on the loader hook worker to process --experimental-loader is moved into lib/internal/modules/esm/worker.js for clarity.
The initialization configuration forceDefaultLoader is split into shouldSpawnLoaderHookWorker and shouldPreloadModules as those can be separate.
--experimental-vm-modules is now processed during pre-execution and no longer part of the initialization of the built-in ESM loader, as it only exposes the vm APIs of ESM, and is unrelated to built-in ESM loading.

- This updates the comments that assume loader hooks must be async - Differentiate the sync/async loader hook paths in naming `#customizations` is now `#asyncLoaderHooks` to make it clear it's from the async APIs. - Differentiate the paths running on the loader hook thread (affects the loading of async other loader hooks and are async) v.s. paths on the main thread calling out to code on the loader hook thread (do not handle loading of other async loader hooks, and can be sync by blocking). - `Hooks` is now `AsyncLoaderHooksOnLoaderHookWorker` - `CustomizedModuleLoader` is now `AsyncLoaderHooksProxiedToLoaderHookWorker` and moved into `lib/internal/modules/esm/hooks.js` as it implements the same interface as `AsyncLoaderHooksOnLoaderHookWorker` - `HooksProxy` is now `AsyncLoaderHookWorker` - Adjust the JSDoc accordingly - Clarify the "loader worker" as the "async loader hook worker" i.e. when there's no _async_ loader hook registered, there won't be this worker, to avoid the misconception that this worker is spawned unconditionally. - The code run on the loader hook worker to process `--experimental-loader` is moved into `lib/internal/modules/esm/worker.js` for clarity. - The initialization configuration `forceDefaultLoader` is split into `shouldSpawnLoaderHookWorker` and `shouldPreloadModules` as those can be separate. - `--experimental-vm-modules` is now processed during pre-execution and no longer part of the initialization of the built-in ESM loader, as it only exposes the vm APIs of ESM, and is unrelated to built-in ESM loading.

nodejs-github-bot · 2025-10-16T12:40:05Z

Review requested:

@nodejs/loaders
@nodejs/startup

joyeecheung · 2025-10-16T12:44:16Z

lib/internal/main/worker_thread.js

    switch (doEval) {
      case 'internal': {
-        // Create this WeakMap in js-land because V8 has no C++ API for WeakMap.
-        internalBinding('module_wrap').callbackMap = new SafeWeakMap();


This is removed as a drive-by, the callbackMap is no longer used after #48510

codecov · 2025-10-16T15:28:26Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.58%. Comparing base (a520a39) to head (f585876).
⚠️ Report is 63 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #60278      +/-   ##
==========================================
+ Coverage   88.56%   88.58%   +0.01%     
==========================================
  Files         704      704              
  Lines      208323   208369      +46     
  Branches    40033    40038       +5     
==========================================
+ Hits       184501   184581      +80     
+ Misses      15854    15823      -31     
+ Partials     7968     7965       -3

Files with missing lines	Coverage Δ
lib/internal/main/worker_thread.js	`94.44% <100.00%> (+0.03%)`	⬆️
lib/internal/modules/esm/hooks.js	`92.00% <100.00%> (+1.32%)`	⬆️
lib/internal/modules/esm/loader.js	`95.79% <100.00%> (-0.39%)`	⬇️
lib/internal/modules/esm/utils.js	`100.00% <100.00%> (ø)`
lib/internal/modules/esm/worker.js	`92.78% <100.00%> (+1.34%)`	⬆️
lib/internal/modules/run_main.js	`97.64% <100.00%> (ø)`
lib/internal/process/pre_execution.js	`97.33% <100.00%> (-0.78%)`	⬇️

... and 44 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2025-10-16T15:47:17Z

CI: https://ci.nodejs.org/job/node-test-pull-request/69761/

GeoffreyBooth · 2025-10-16T15:49:11Z

lib/internal/modules/esm/hooks.js

+/**
+ * Interface for classes that implement asynchronous loader hooks that can be attached to the ModuleLoader
+ * via `ModuleLoader.#setAsyncLoaderHooks()`.
+ * @typedef {object} AsyncLoaderHooks


Nit: the reason for the “customization“ and “customization hooks“ names is to avoid confusion where some might think that these hooks only affect loading, when they also impact resolution and other things. It hasn’t been a priority to update the code, but as long as we’re adding new code and defining new names, I think the “customization“ name is more informative and correct.

Suggested change

* @typedef {object} AsyncLoaderHooks

* @typedef {object} AsyncCustomizationHooks

And elsewhere that the code uses “loader hook“ that this branch touches.

In that case should AsyncLoaderHooksOnLoaderHookWorker be AsyncCustomizationHooksOnCustomizationHookWorker? That feels rather lengthy..

I feel that "loader hooks" contain already ample information about it: it's loader, which implies it's not just "loading" phase but everything you'd see in loader.js and the ModuleLoader class. I can see that "load" would be ambuiguous, but "loader" isn't, and "hooks" implies customizations (or the hooks aren't necessarily for customizations - they can also be for observations).

It's just a nit, do with it as you will. AsyncCustomizationHooksOnCustomizationHookWorker is the same number of words as with Loader, it's just a longer word. It's a mouthful in either version. Perhaps just AsyncOffThreadCustomizationHooks?

Another name instead of “customization hooks“ is “module hooks“ which is just as descriptive and the same number of letters as “loader hooks”.

I appreciate the brainstorm ;) though to me "foo hooks" means "it's hooking into foo/hook placed in foo", so I'd have some questions when I see "module hooks" - is it hooking into the code of user modules/hook placed in the modules? While it can be used in such a way that inject code into user modules, that is more of the output, instead of the process. "loader hooks" seem more intuitive because, there's a ModuleLoader in loader.js next to it, which either new AsyncLoaderHooksProxiedToLoaderHookWorker itself, or get a new AsyncLoaderHooksOnLoaderHookWorker passed into it, and the two classes have methods that are hooking into the methods of ModuleLoader with the same name, so that makes a lot of sense.

(I thought about just calling it async hooks but then unfortunately that combination might be a bit too well known as something completely unrelated in Node.js..)

nodejs-github-bot · 2025-10-17T10:55:37Z

CI: https://ci.nodejs.org/job/node-test-pull-request/69773/

nodejs-github-bot · 2025-10-19T20:03:13Z

CI: https://ci.nodejs.org/job/node-test-pull-request/69823/

nodejs-github-bot · 2025-10-20T10:55:44Z

CI: https://ci.nodejs.org/job/node-test-pull-request/69828/

nodejs-github-bot · 2025-10-23T13:42:25Z

Landed in b19525a

- This updates the comments that assume loader hooks must be async - Differentiate the sync/async loader hook paths in naming `#customizations` is now `#asyncLoaderHooks` to make it clear it's from the async APIs. - Differentiate the paths running on the loader hook thread (affects the loading of async other loader hooks and are async) v.s. paths on the main thread calling out to code on the loader hook thread (do not handle loading of other async loader hooks, and can be sync by blocking). - `Hooks` is now `AsyncLoaderHooksOnLoaderHookWorker` - `CustomizedModuleLoader` is now `AsyncLoaderHooksProxiedToLoaderHookWorker` and moved into `lib/internal/modules/esm/hooks.js` as it implements the same interface as `AsyncLoaderHooksOnLoaderHookWorker` - `HooksProxy` is now `AsyncLoaderHookWorker` - Adjust the JSDoc accordingly - Clarify the "loader worker" as the "async loader hook worker" i.e. when there's no _async_ loader hook registered, there won't be this worker, to avoid the misconception that this worker is spawned unconditionally. - The code run on the loader hook worker to process `--experimental-loader` is moved into `lib/internal/modules/esm/worker.js` for clarity. - The initialization configuration `forceDefaultLoader` is split into `shouldSpawnLoaderHookWorker` and `shouldPreloadModules` as those can be separate. - `--experimental-vm-modules` is now processed during pre-execution and no longer part of the initialization of the built-in ESM loader, as it only exposes the vm APIs of ESM, and is unrelated to built-in ESM loading. PR-URL: #60278 Reviewed-By: Geoffrey Booth <[email protected]>

nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Oct 16, 2025

joyeecheung commented Oct 16, 2025

View reviewed changes

joyeecheung mentioned this pull request Oct 16, 2025

Always run async loader hooks synchronously from the non-loader thread #59666

Open

GeoffreyBooth approved these changes Oct 16, 2025

View reviewed changes

JakobJingleheimer self-requested a review October 16, 2025 16:38

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 19, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 19, 2025

joyeecheung added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 23, 2025

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 23, 2025

nodejs-github-bot merged commit b19525a into nodejs:main Oct 23, 2025
69 of 70 checks passed

	* @typedef {object} AsyncLoaderHooks
	* @typedef {object} AsyncCustomizationHooks

Uh oh!

module: refactor and clarify async loader hook customizations #60278

module: refactor and clarify async loader hook customizations #60278

Conversation

joyeecheung commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Oct 16, 2025

Uh oh!

joyeecheung Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nodejs-github-bot commented Oct 16, 2025

Uh oh!

GeoffreyBooth Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

joyeecheung Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GeoffreyBooth Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joyeecheung Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Oct 17, 2025

Uh oh!

nodejs-github-bot commented Oct 19, 2025

Uh oh!

nodejs-github-bot commented Oct 20, 2025

Uh oh!

Uh oh!

nodejs-github-bot commented Oct 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

joyeecheung commented Oct 16, 2025 •

edited

Loading

codecov bot commented Oct 16, 2025 •

edited

Loading

joyeecheung Oct 16, 2025 •

edited

Loading

GeoffreyBooth Oct 16, 2025 •

edited

Loading

joyeecheung Oct 16, 2025 •

edited

Loading